-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement exercise dominoes #726
Conversation
ceb163f
to
43c4d4a
Compare
exercises/dominoes/dominoes_test.py
Outdated
from dominoes import chain | ||
|
||
|
||
# test cases adapted from `x-common//canonical-data.json` @ version: 1.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a new format for the version string to reflect the name-change from x-common
to problem-specifications
that happened a while back (#784).
# Tests adapted from `problem-specifications//canonical-data.json` @ v1.0.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually noticed that these tests weren't even based off of 1.0.0; it was the current version, 2.0.0. This has been updated.
exercises/dominoes/README.md
Outdated
which don't have a neighbour (the first and last stone) match each other. | ||
|
||
For example given the stones `21`, `23` and `13` you should compute something | ||
like `12 23 31` or `32 21 13` or `13 32 21` etc, where the first and last numbers are the same. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find it weird that they are specified like two digit numbers when these numbers are supposed to be separate things - I definitely prefer your implementation with tuples. Can you update the README to use tuples instead? It would be worth submitting this change to problem-specifications as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exercises/dominoes/README.md
Outdated
|
||
Some test cases may use duplicate stones in a chain solution, assume that multiple Domino sets are being used. | ||
|
||
### Submitting Exercises |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move this heading to level 2 (##
) to save me doing it in #950?
@N-Parsons I've addressed your review comments and applied the latest changes to the description. |
Sorry for taking a while to get back to this, @cmccandless! |
* add dominoes/README.md * Add test cases and example solution for dominoes * add dominoes to config.json * dominoes: add check for name == "__main__" * dominoes: update canonical data version and formatting fixes in README * dominoes: update README to latest description RE: exercism/problem-specifications#972
* add dominoes/README.md * Add test cases and example solution for dominoes * add dominoes to config.json * dominoes: add check for name == "__main__" * dominoes: update canonical data version and formatting fixes in README * dominoes: update README to latest description RE: exercism/problem-specifications#972
TODO:
Resolves #749